Skip to content

Conversation

@cjappl
Copy link
Contributor

@cjappl cjappl commented Oct 7, 2024

In an upcoming review, this code block in rtsan_assertions is going to get a lot messier.

Getting this in as a quick early refactor

@llvmbot
Copy link
Member

llvmbot commented Oct 7, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Chris Apple (cjappl)

Changes

In an upcoming review, this code block in rtsan_assertions is going to get a lot messier.

Getting this in as a quick early refactor


Full diff: https://github.com/llvm/llvm-project/pull/111438.diff

2 Files Affected:

  • (modified) compiler-rt/lib/rtsan/rtsan_assertions.h (+1-2)
  • (modified) compiler-rt/lib/rtsan/rtsan_context.h (+12)
diff --git a/compiler-rt/lib/rtsan/rtsan_assertions.h b/compiler-rt/lib/rtsan/rtsan_assertions.h
index 1a653d198ab88b..4646e750b6796e 100644
--- a/compiler-rt/lib/rtsan/rtsan_assertions.h
+++ b/compiler-rt/lib/rtsan/rtsan_assertions.h
@@ -21,9 +21,8 @@ template <typename OnViolationAction>
 void ExpectNotRealtime(Context &context, OnViolationAction &&OnViolation) {
   CHECK(__rtsan_is_initialized());
   if (context.InRealtimeContext() && !context.IsBypassed()) {
-    context.BypassPush();
+    ScopedBypass sb{context};
     OnViolation();
-    context.BypassPop();
   }
 }
 
diff --git a/compiler-rt/lib/rtsan/rtsan_context.h b/compiler-rt/lib/rtsan/rtsan_context.h
index cb0c2eb0a5e0d7..2d906259e1fe35 100644
--- a/compiler-rt/lib/rtsan/rtsan_context.h
+++ b/compiler-rt/lib/rtsan/rtsan_context.h
@@ -35,5 +35,17 @@ class Context {
   int bypass_depth_{0};
 };
 
+class ScopedBypass {
+public:
+  [[nodiscard]] explicit ScopedBypass(Context &context) : context_(context) {
+    context_.BypassPush();
+  }
+
+  ~ScopedBypass() { context_.BypassPop(); }
+
+private:
+  Context &context_;
+};
+
 Context &GetContextForThisThread();
 } // namespace __rtsan

@cjappl cjappl merged commit 10d4306 into llvm:main Oct 7, 2024
6 of 7 checks passed
@cjappl cjappl deleted the ScopedDisabler branch October 7, 2024 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants